-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bootstrap right-to-left integration #178
base: master
Are you sure you want to change the base?
Bootstrap right-to-left integration #178
Conversation
Excellent - thanks! Are you happy to have these changes released under the MIT license? I'm going to be updating this file to be SCSS based in the next month or so which will make this kind of support much easier I think - better Bootstrap integration for all DataTables extensions is on my to do list - it is a mess atm! |
Sure. If you need me to add my name to a CLA, etc., just point me at it. |
I'm happy with the above as acceptance. That's all the CLA would say anyway :-). I'll try it out shortly and merge it in after that. |
Your RTL integration is rather more complete that mine from DataTables' core stylesheet! I've got it so that it simply handles RTL without actually breaking anything (for example all the layout components stay where they are), while you've gone for the complete flip. I need to have a little think on what the correct approach here is so I can harmonise the two. I'm quite tempted to suggest that there should be an RTL stylesheet since the majority of users won't need these extra styles. Would be interested in your own thoughts (are you a native RTL language speaker / reader?). |
RTL pages need to be exact mirrors of the LTR pages. Here is an example using dataTables with jQuery-UI: https://dev.webtrees.net/demo-dev/indilist.php?alpha=%2C&ged=demo&lang=ar The localisation of the pagination buttons is currently broken, but we're moving to Bootstrap so no point working on it further.
I'm not, but I've spent 10 years on multi-lingual projects, and work closely with native Hebrew and Arabic speakers. I think I've come across most RTL issues in that time... The issues I've reported came directly from our Hebrew translator/tester.
My experience of using extra RTL stylesheets is that they are harder to keep up-to-date and maintain. If you have a single stylesheet where each "left/right" rule is immediately followed by its RTL cousin, things are much easier in the long term. It's also easier for users/developers to have a single file to include. A module/library written using datatables plus the plugin won't need to know if it is being used in a project that has RTL. IIRC, you were considering a move to SASS/LESS? If so, try to starting thinking in terms of "start" and "end", rather than "left" and "right". Also, if you have a spare few hours, start reading twbs/bootstrap#13564 and follow the links... ;-) |
Great feedback - thanks. The downside to including RTL support in the main file is that it adds dead code weight for the majority of users. As a library author I feel obliged to keep the file size as small as possible - but that is a balance between file size and features.
Certainly am. Current plan is to move the Bootstrap (Foundation, etc) integration out of this repo and into the individual extensions repos. More files, but with a download builder (coming soon) that shouldn't be an issue and will help improve how easy it is to create and test individual use cases. RTL could be a checkbox option for that, although I must admit I don't fancy doubling the number of CSS files... |
You know your project's audience better than I do, but the counter-arguements are
|
Let me get back to you on this. I'll have a read though of some of the BS thread comments and links. It might be a little while before I can do so due to the amount of stuff going on, but this is something I would like to have supported. Just how exactly and what the best way of doing it is remains to be defined. |
@DataTables @fisharebest there's #51 which has been sitting in the pull requests for about a year and achieves the same thing plus FontAwesome and Glyphicons integration... |
FYI, I have updated/rebased this for the latest (1.10.7) code. @elad - IMHO, your pull request does too many different/unrelated things. If a pull-request contains three changes, it is difficult for a project owner to merge one, reject one and discuss the other... |
This is a pull-request for #177 - RTL support for the bootstrap3 integration.
The only changes are to add new CSS rules with a
[dir=rtl]
prefix, so it shouldn't break any existing pages.Bootstrap itself is heavily LTR-only, and it is assumed that some fixup for this is already present. (https://github.com/morteza/bootstrap-rtl is widely used for this purpose.)
I've split the PR into two commits (the second one just fixes whitespace) for easy reviewing.
I've tested this on the latest Firefox, Chrome, and IE8/9/10.
I have not tested the changes to
DTTT_print_info
, because I can't work out where it is used.The
DTFC_RightHeadWrapper/DTFC_LeftHeadWrapper
rule resets 3 of the 4 corners. Again, I can't work out where this is used, but the changes should work.